Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SharedCache] Cache references to type libraries #6195

Closed
wants to merge 1 commit into from

Conversation

WeiN76LQh
Copy link

@WeiN76LQh WeiN76LQh commented Nov 25, 2024

SharedCache::FindSymbolAtAddrAndApplyToAddr significantly impacts loading and analysis of DSC libraries. Its a hot path and the main cause of its slow execution time is the loading of a type library by name. By caching references to type libraries this function spends 90%+ less time executing.

The cache is a global because SharedCache gets destroyed and re-created alot so I think this is the best place to store cached references. The downside to this is it'll cause a memory leak as I map entries using the view session ID. Entries won't be removed once the view is closed as I'm unsure how to do this. That said I think the performance benefits far outway the potential memory leak which will likely have no noticeable affect on the user experience or BN memory consumption.

`SharedCache::FindSymbolAtAddrAndApplyToAddr` significantly impacts loading and analysis of DSC libraries. Its a hot path and the main cause of its slow execution time is the loading of a type library by name. By caching references to type libraries this function spends 90%+ less time executing.

The cache is a global because `SharedCache` gets destoryed and re-created alot so I think this is the best place to store cached references. The downside to this is it'll cause a memory leak as I map entries using the view session ID. Entries won't be removed once the view is closed as I'm unsure how to do this. That said I think the performance benefits far outway the potential memory leak which will likely have no noticeable affect on the user experience of BN memory consumption.
@bdash
Copy link
Contributor

bdash commented Nov 25, 2024

I was thinking about this after you mentioned it was a bottleneck on bdash#2. I noticed that the way that some of the current view-specific state, including the mutexes, are accessed has a data race. I ended up implementing a very similar change (#6196) that fixes those data races and is able to free memory when the shared cache is closed.

@WeiN76LQh
Copy link
Author

Closed in favour of #6196

@WeiN76LQh WeiN76LQh closed this Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants